[10.x] Fix Http global middleware for queue, octane, and dependecy injection #47915
Conversation
timacdonald
changed the title
|
Just found this bug today... was about to open an issue. Thanks for the fix! |
|
No troubles! |
|
This breaks the testing though! I am faking an http response of an URL in a test. This fake is written in a base test class. This is used by multiple other test cases. Some of these test cases implement another fake http response of the same URL. Simplified code to reproduce: <?php
use Illuminate\Support\Facades\Http;
Route::get('/', function (){
Http::fake([
'https://laravel.com' => Http::response('OK', 200)
]);
dump(Http::get('https://laravel.com')->body());
Http::fake([
'https://laravel.com' => Http::response('Internal Server Error', 500)
]);
dump(Http::get('https://laravel.com')->body());
});On the Laravel Playground: https://laravelplayground.com/#/snippets/0f8268bf-57d5-4709-ac54-e24965ec4c22 The same issue is being faced by others as well: https://laracasts.com/discuss/channels/laravel/overwrite-fake-response-in-http-client?page=1&replyId=762080 The solution suggested (swap) - it used to work until this PR got merged. That solution broke with this PR. The test case (with multiple http calls faked) are run in a single request. Singletons cause an issue in testing because of that. |
|
Responded in #49398 |
fixes #47738
We added the ability to add global middleware to the Http client.
Because Facades are cached for the duration of a request, this works as expected for each Http request.
On the queue / in octane however, these middleware are not applied.
This is because the underlying
Factoryis not a singleton, so theFactoryinstance is refreshed on each job execution / octane request. TheFactoryinstance is where the middleware is stored.This PR binds the
Factoryas a singleton. Excluding testing infrastructure, the only state theFactorymaintains is the global middleware and the event service provider. As the event service provider is also a singleton, we shouldn't need to refresh it manually for Octane.In addition to the queue and Octane fixes, this fix also means that the requests made by the
NotPwnedVerifierwill now pass through the global middleware, which I believe is the expected behaviour.This is because the
NotPwnedVerifierinjects theFactoryvia the constructor, rather than accessing the Facade.This also applies to any user-land code that is injecting the
Factoryclass, which I also believe is the expected behaviour.